-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wip/hoop chart #173
Wip/hoop chart #173
Conversation
@webwarrior-ws good work! Can you enable GitHubActions in your fork please? |
src/GWallet.Frontend.XF/HoopChart.fs
Outdated
|
||
let balanceTagLabel = | ||
Label( | ||
Text = "Amount Balance", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@webwarrior-ws actually let's have the same text as the old label: Total Assets:\n
Changed label text. CI job failure seems to be unrelated. |
True, it was unrelated, but do you mind rebasing your PR? I fixed the CI issue in |
I must have done something wrong, because now I can't push:
|
Did you forget the flag --force? |
If that doesn't do it; it seems it's the way you cloned your fork? I recommend you to use SSH, not weird OAuth stuff. |
OK, now with SSH it worked |
I said rebase, not merge :( When rebasing, there should not be any merge commits on top of your branch. |
So now what? Do I checkout or reset to latest commit that is not mine? |
IMO you should first delete your branch locally, then checkout commit 9e70c43 and create a new branch from it (same name as the one that this PR is targetting), then cherry-pick your commits from github, then push with --force. |
Finally, I had time to test it today (sorry, yesterday I was in the eye-doctor and they gave me something that made my sight blurry). Code-wise, the proposed PR looks fine; integration seems clean. But I have this rendering problem: As you can see, the circle is too big, and causes the currency frames to need scrolling. Ideally the circle shouldn't be rendered as big and everything should fit. Do you know if this is easy to fix? |
Probably not. |
Exactly! But the idea is that the circle only gets big when it needs to (because of a big label). In the case of the screenshot, the amount is not very long, so the circle should not need to be that big. |
… when balance is empty
Addresses #172 (Better design for cheese chart (so that text is inside it))